Skip to contentSkip to conversation

Use absolute offset for embedding area in grub-setup (i386-pc)

PATCH In review Grégoire Sutre 2 participants 2 messages Jun 5, 2010 – Jun 7, 2010

lore
Share & tools
lore

Grégoire Sutre

author
#0 lore raw

PATCH 1/1 Use absolute offset for embedding area in grub-setup (i386-pc)

Hi,

The attached patch uses grub_partition_get_start (p) instead of p->start in grub-setup's iteration over partitions. While we are at it, also use grub_partition_get_len (p) instead of p->len.

An alternative would be to assume that partitions are properly nested, and therefore iterate only over the top-level ones (i.e. those with p->parent == NULL). In that case, we could equivalently use p->start.

In the check for non-empty embedding area, the patch uses <= instead of == (as we may have embed_region.start == 1 && embed_region.end == 0).

Comments?

Grégoire

=== modified file 'ChangeLog'

ChangeLog +5 -0
@@ -1,3 +1,8 @@
+2010-06-05  Grégoire Sutre  <[email protected]>
+
+	* util/i386/pc/grub-setup.c (setup): Use absolute offsets for start of
+	embedding area.  Use <= instead of == when checking for non-emptyness.
+
 2010-06-03  Colin Watson  <[email protected]>
 
 	* INSTALL: Document several build requirements for optional features

=== modified file 'util/i386/pc/grub-setup.c'

i386/pc/grub-setup.c +5 -5
@@ -126,8 +126,8 @@ setup (const char *dir,
       /* There's always an embed region, and it starts right after the MBR.  */
       embed_region.start = 1;
 
-      if (embed_region.end > p->start)
-	embed_region.end = p->start;
+      if (embed_region.end > grub_partition_get_start (p))
+	embed_region.end = grub_partition_get_start (p);
 
       return 0;
     }

@@ -147,8 +147,8 @@ setup (const char *dir,
       /* If there's an embed region, it is in a dedicated partition.  */
       if (! memcmp (&gptdata.type, &grub_gpt_partition_type_bios_boot, 16))
 	{
-	  embed_region.start = p->start;
-	  embed_region.end = p->start + p->len;
+	  embed_region.start = grub_partition_get_start (p);
+	  embed_region.end = grub_partition_get_start (p) + grub_partition_get_len (p);
 
 	  return 1;
 	}

@@ -361,7 +361,7 @@ setup (const char *dir,
   else
     grub_util_error (_("No DOS-style partitions found"));
 
-  if (embed_region.end == embed_region.start)
+  if (embed_region.end <= embed_region.start)
     {
       if (! strcmp (dest_partmap, "msdos"))
 	grub_util_warn (_("This msdos-style partition label has no post-MBR gap; embedding won't be possible!"));
#1 lore raw
Quoted 5 lines · Grégoire Sutre

Hi,

The attached patch uses grub_partition_get_start (p) instead of p->start in grub-setup's iteration over partitions. While we are at it, also use grub_partition_get_len (p) instead of p->len.

Go ahead

Quoted 19 lines

An alternative would be to assume that partitions are properly nested, and therefore iterate only over the top-level ones (i.e. those with p->parent == NULL). In that case, we could equivalently use p->start.

In the check for non-empty embedding area, the patch uses <= instead of == (as we may have embed_region.start == 1 && embed_region.end == 0).

Comments?

Grégoire

_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel

Signature · 2 lines
Regards
Vladimir 'φ-coder/phcoder' Serbinenko